Harden DCP chat transforms and clarify compacted output#510
Conversation
Replace the opaque '[Old tool result content cleared]' placeholder with '[Tool output compacted locally; original in session storage.]' to make it clear that compaction is a local optimization, not a provider failure. This reduces confusion when users see compacted output in their sessions.
Apply three defensive patches from 2026-04-16 session hardening: 1. hooks.ts: Wrap createChatMessageTransformHandler body in try/catch so any DCP transform failure degrades safely instead of aborting the session. 2. message-ids.ts: Replace hard-throw on alias exhaustion with graceful degradation (return empty string). This prevents session crashes when message volume exceeds the referenceable range. 3. token-utils.ts: (already committed) Improve compacted tool output placeholder message for clarity. These are fail-open defensive changes that reduce ghost/hard-stop behavior when DCP encounters edge cases.
| } | ||
|
|
||
| export const COMPACTED_TOOL_OUTPUT_PLACEHOLDER = "[Old tool result content cleared]" | ||
| export const COMPACTED_TOOL_OUTPUT_PLACEHOLDER = "[Tool output compacted locally; original in session storage.]" |
There was a problem hiding this comment.
This is a string that is set by opencode for messages that they prune, did they change it to yours?
There was a problem hiding this comment.
Reverted this to the original opencode placeholder string.
| throw new Error( | ||
| `Message ID alias capacity exceeded. Cannot allocate more than ${formatMessageRef(MESSAGE_REF_MAX_INDEX)} aliases in this session.`, | ||
| ) | ||
| // Graceful degradation: stop allocating aliases instead of hard-throwing. |
There was a problem hiding this comment.
This doesn't make much sense we already allow for 9999 messages and if somehow a session has more than that I would rather see an error than for things to silently fail
There was a problem hiding this comment.
Agreed. Restored the explicit capacity error and updated the regression coverage to assert the throw instead of silently stopping.
| if (state.sessionId) { | ||
| await logger.saveContext(state.sessionId, output.messages) | ||
| } | ||
| } catch (err) { |
There was a problem hiding this comment.
I think this would still allow these functions to mutate messages, if there is an error thrown we should probably use a deep copy or something like that instead of a partially completed mutation wherever the error occured
There was a problem hiding this comment.
Good point. I changed the fail-open path to run transforms against a cloned message array and only commit back to output.messages after the pipeline succeeds, preserving the original array identity with splice. If a transform throws, the original messages are left untouched. Added regression coverage for late transform failure, clone failure, and the disabled-subagent pre-skip path.
2 participants
Summary
Verification
npm testupstream/devwith no conflictsNotes